Skip to content

Issue-6654 : Skip malformed PURLs instead of assigning to empty resolver#6679

Open
sahibamittal wants to merge 1 commit into
DependencyTrack:mainfrom
sahibamittal:issue-6654-skip-malformed-purl-resolution
Open

Issue-6654 : Skip malformed PURLs instead of assigning to empty resolver#6679
sahibamittal wants to merge 1 commit into
DependencyTrack:mainfrom
sahibamittal:issue-6654-skip-malformed-purl-resolution

Conversation

@sahibamittal

@sahibamittal sahibamittal commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Description

When a PURL fails to parse, skip it entirely rather than assigning it to an empty resolver. This prevents unnecessary resolution attempts and reduces log spam from recurring WARN messages on each cycle. Malformed PURLs are a data quality issue and should not be processed.

Added test verifying malformed PURLs are excluded from candidates and not reselected across workflow runs.

Addressed Issue

Closes #6654

Additional Details

Checklist

  • I have read and understand the contributing guidelines
  • This PR fixes a defect, and I have provided tests to verify that the fix is effective
  • This PR implements an enhancement, and I have provided tests to verify that it works as intended
  • This PR introduces changes to the database model, and I have updated the migration changelog accordingly
  • This PR introduces new or alters existing behavior, and I have updated the documentation accordingly
  • This PR is a substantial change (per the ADR criteria), and I have added an ADR under docs/adr/

When a PURL fails to parse, skip it entirely rather than assigning it to an empty resolver. This prevents unnecessary resolution attempts and reduces log spam from recurring WARN messages on each cycle. Malformed PURLs are a data quality issue and should not be processed.

Added test verifying malformed PURLs are excluded from candidates and not reselected across workflow runs.

Signed-off-by: Sahiba Mittal <sahiba.mittal@citi.com>
@owasp-dt-bot

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@codacy-production

codacy-production Bot commented Jul 13, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Coverage 100.00% diff coverage · +0.01% coverage variation

Metric Results
Coverage variation +0.01% coverage variation (-1.00%)
Diff coverage 100.00% diff coverage (70.00%)

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (13d5b2d) 42825 37196 86.86%
Head commit (174ad48) 42822 (-3) 37199 (+3) 86.87% (+0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#6679) 1 1 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Comment on lines +76 to 79
// Malformed PURL cannot be resolved and is data quality issue.
// Skip to avoid resolution attempts and spamming WARN logs on every cycle.
LOGGER.debug("Skipping malformed PURL '{}'", purlStr, e);
continue;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply skipping these would cause them to always remain eligible for resolution because no result is persisted for them.

The previous behavior assigned the "empty" resolver, which then later trivially persists empty results:

if (resolverName.isEmpty()) {
// The resolver name is empty when no resolver exists that
// supports the given batch of PURLs. In this case, simply
// store empty results from preventing these PURLs to become
// resolution candidates again in the next batch.
final var buffer = new ResultBuffer();
for (final String purlStr : purlStrings) {
buffer.addEmptyResult(purlStr);
}
buffer.flush();
return null;
}

This prevents the next FetchPackageMetadataResolutionCandidatesRes iteration from discovering entries with invalid PURLs again.

By skipping this mechanism, ResolvePackageMetadataWorkflow would effectively loop forever and never terminate. It basically breaks the exit condition, i.e. this query returning no results:

SELECT DISTINCT c."PURL"
FROM "COMPONENT" c
WHERE c."PURL" IS NOT NULL
AND NOT EXISTS (
SELECT 1
FROM "PACKAGE_ARTIFACT_METADATA" pam
JOIN "PACKAGE_METADATA" pm
ON pm."PURL" = pam."PACKAGE_PURL"
WHERE pam."PURL" = c."PURL"
AND pm."RESOLVED_AT" > NOW() - INTERVAL '24 hours'
)
ORDER BY c."PURL"
LIMIT :limit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing the same malformed PURL being logged on every resolution cycle with the current implementation. So the empty resolver path is not actually preventing it from being rediscovered. Same malformed PURL should not be processed again after the first successful workflow execution.
I'll check whether the empty result is being persisted and why FetchPackageMetadataResolutionCandidatesRes is still selecting it.

@sahibamittal sahibamittal Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay found it, I noticed that for the malformed PURL, ResultBuffer.addEmptyResult() returns immediately without persisting empty results because PurlUtil.silentPurl(purlStr) returns null:

final PackageURL purl = PurlUtil.silentPurl(purlStr);
if (purl == null) {
    return;
}

As a result, no PackageMetadata or PackageArtifactMetadata records are written, and the same component is selected again by FetchPackageMetadataResolutionCandidatesActivity on every cycle. This needs to be fixed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model currently requires a valid PURL since the identifiers are PackageURL objects. We'd need a way to pass empty results for broken PURLs, ideally without loosening the types (i.e. changing from PackageURL to String) too much.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OR instead of persisting malformed PURLs (empty results), should it not just reject or sanitize them during BOM ingestion so they never reach the metadata resolver?

Also, currently we decide resolution state based on metadata tables if data exists. How about persisting the outcome of metadata resolution separately from the metadata itself. Say, PACKAGE_METADATA_RESOLUTION to record resolution state (e.g. SUCCESS, EMPTY, INVALID_PURL etc) keyed by the raw PURL. This way metadata becomes just the payload.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OR instead of persisting malformed PURLs (empty results), should it not just reject or sanitize them during BOM ingestion so they never reach the metadata resolver?

Hmmm that is already happening:

if (cdxComponent.getPurl() != null) {
try {
final var purl = new PackageURL(cdxComponent.getPurl());
component.setPurl(purl);
component.setPurlCoordinates(silentPurlCoordinatesOnly(purl));
} catch (MalformedPackageURLException e) {
LOGGER.debug("Encountered invalid PURL", e);
}
}

Which begs the question how you could still see invalid PURLs reaching the metadata resolver. I wonder if it's related to an encoding bug in the packageurl-java library (see #6383 where it came up recently). Does that error pattern match what you're observing in your instance?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked none of repository records have authenticationRequired set to null. Error is due to malformed escape pair in invalid PURL during metadata resolution:

image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, no I meant the PURL encoding issue that's also mentioned there.

pkg:nuget/serilog.sinks.file%A%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%20%205.0.0@5.0.0 is likely not how the PURL looked when it entered the system, the packageurl-java lib may have borked it when converting it from PackageURL to String. That would explain why it survived the import:

  • First new PackageURL(cdxComponent.getPurl()) works fine
  • Value is persisted to DB using PackageURL.canonicalize(), which borks the encoding
  • new PackageURL(<value-from-db>) now fails

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing negative-cache for Malformed PURLs

3 participants